feat: add HTTP proxy support via environment variables#622
feat: add HTTP proxy support via environment variables#622jpoehnelt merged 4 commits intogoogleworkspace:mainfrom
Conversation
When http_proxy/https_proxy/all_proxy environment variables are set, use reqwest (which natively supports proxy) for token refresh instead of yup-oauth2's hyper-based client (which doesn't support proxy). This enables gws to work in environments that require HTTP proxy to access Google APIs (e.g., users in China). Changes: - Cargo.toml: Enable reqwest's default features including proxy support - src/auth.rs: Add proxy-aware token refresh using reqwest as fallback Fixes googleworkspace#422
When proxy env vars are set, use a custom OAuth flow with reqwest for token exchange instead of yup-oauth2's hyper-based client. Changes to auth_commands.rs: - Add login_with_proxy_support() for proxy-aware OAuth login - Add exchange_code_with_reqwest() for token exchange via reqwest - Detect proxy env vars and choose appropriate flow
🦋 Changeset detectedLatest commit: ec20bdf The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces proxy support for OAuth authentication flows in the Google Workspace CLI. This is achieved by adding the socks feature to the reqwest dependency and implementing new functions that leverage reqwest for proxy-aware token refreshing and authorization code exchange. The existing authentication logic has been updated to conditionally use these new proxy-aware flows when proxy environment variables are detected. Unit tests for the new proxy-related utilities have also been added. The review comments suggest that creating a new reqwest::Client for each function call is inefficient and recommend using a single, shared client instance across the application to improve performance.
|
Addressed the reqwest client reuse feedback by adding a shared client helper backed by std::sync::OnceLock in crates/google-workspace/src/client.rs and switching the proxy auth flows to use it. |
|
/gemini review |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #622 +/- ##
==========================================
- Coverage 71.09% 70.88% -0.21%
==========================================
Files 44 44
Lines 20403 20642 +239
==========================================
+ Hits 14506 14633 +127
- Misses 5897 6009 +112 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request significantly improves proxy support for OAuth authentication flows within the CLI. It introduces conditional logic to use reqwest for token exchange and refresh when proxy environment variables are detected, addressing limitations with the yup-oauth2 library's hyper-based client. Key changes include adding the socks feature to reqwest, refactoring OAuth login and token refresh into dedicated proxy-aware functions, and introducing a shared reqwest::Client instance for efficient HTTP requests. A review comment highlighted an improvement opportunity regarding the robustness of temporary file cleanup during the OAuth flow, suggesting the use of an RAII guard to prevent sensitive data leaks in case of panics.
| let temp_path = config_dir.join("credentials.tmp"); | ||
| let _ = std::fs::remove_file(&temp_path); | ||
|
|
||
| let result = async { | ||
| let auth = yup_oauth2::InstalledFlowAuthenticator::builder( | ||
| secret, | ||
| yup_oauth2::InstalledFlowReturnMethod::HTTPRedirect, | ||
| ) | ||
| .with_storage(Box::new(crate::token_storage::EncryptedTokenStorage::new( | ||
| temp_path.clone(), | ||
| ))) | ||
| .force_account_selection(true) | ||
| .flow_delegate(Box::new(CliFlowDelegate { login_hint: None })) | ||
| .build() | ||
| .await | ||
| .map_err(|e| GwsError::Auth(format!("Failed to build authenticator: {e}")))?; | ||
|
|
||
| let scope_refs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect(); | ||
| let token = auth | ||
| .token(&scope_refs) | ||
| .await | ||
| .map_err(|e| GwsError::Auth(format!("OAuth flow failed: {e}")))?; | ||
|
|
||
| let access_token = token | ||
| .token() | ||
| .ok_or_else(|| GwsError::Auth("No access token returned".to_string()))? | ||
| .to_string(); | ||
| let refresh_token = read_refresh_token_from_cache(&temp_path)?; | ||
|
|
||
| Ok((access_token, refresh_token)) | ||
| } | ||
| .await; | ||
|
|
||
| let _ = std::fs::remove_file(&temp_path); | ||
| result |
There was a problem hiding this comment.
The manual cleanup of the temporary file credentials.tmp is not robust against panics. If a panic occurs during the OAuth flow, the temporary file containing sensitive token data (though encrypted) might be left on disk.
To ensure the file is always cleaned up, you can use an RAII guard. This also simplifies the code by removing the explicit cleanup at the end and the unnecessary async block.
let temp_path = config_dir.join("credentials.tmp");
// Clean up any stale file from a previous run.
let _ = std::fs::remove_file(&temp_path);
// RAII guard to ensure the temporary file is removed on scope exit, even on panics.
struct TempFileGuard<'a>(&'a Path);
impl<'a> Drop for TempFileGuard<'a> {
fn drop(&mut self) {
let _ = std::fs::remove_file(self.0);
}
}
let _guard = TempFileGuard(&temp_path);
let auth = yup_oauth2::InstalledFlowAuthenticator::builder(
secret,
yup_oauth2::InstalledFlowReturnMethod::HTTPRedirect,
)
.with_storage(Box::new(crate::token_storage::EncryptedTokenStorage::new(
temp_path.clone(),
)))
.force_account_selection(true)
.flow_delegate(Box::new(CliFlowDelegate { login_hint: None }))
.build()
.await
.map_err(|e| GwsError::Auth(format!("Failed to build authenticator: {e}")))?;
let scope_refs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
let token = auth
.token(&scope_refs)
.await
.map_err(|e| GwsError::Auth(format!("OAuth flow failed: {e}")))?;
let access_token = token
.token()
.ok_or_else(|| GwsError::Auth("No access token returned".to_string()))?
.to_string();
let refresh_token = read_refresh_token_from_cache(&temp_path)?;
Ok((access_token, refresh_token))3 participants
Summary
When
http_proxy/https_proxy/all_proxyenvironment variables are set, usereqwest(which supports proxy env vars natively) for token refresh and the auth login token exchange instead of relying only onyup-oauth2'shyper-based client.This enables
gwsto work in environments that require an HTTP proxy to access Google APIs.Supersedes closed PR #423 after rebasing onto the cargo workspace refactor.
Changes
crates/google-workspace-cli/src/auth.rsreqwestcrates/google-workspace-cli/src/auth_commands.rscrates/google-workspace-cli/Cargo.tomlreqwestconfigured with SOCKS support in the workspace crate.changeset/proxy-support-review-fixes.mdTesting
cargo clippy -p google-workspace-cli -- -D warningscargo test -p google-workspace-cli auth::tests::cargo test -p google-workspace-cli auth_commands::tests::